Closed
Bug 1400979
Opened 8 years ago
Closed 8 years ago
test-verify fails on Linux x64 CCov when no tests run
Categories
(Testing :: General, defect)
Testing
General
Tracking
(firefox57 fixed)
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: gbrown, Assigned: gbrown)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 1 obsolete file)
649 bytes,
patch
|
sparky
:
review+
|
Details | Diff | Splinter Review |
1.30 KB,
patch
|
sparky
:
review+
|
Details | Diff | Splinter Review |
test-verify runs fine on Linux x64 CCov when supported test files have been modified and test verification is triggered. However, when no test files have been modified, test-verify completes without running any tests (as I would expect) and that seems to trigger a failure in the coverage code, like:
https://public-artifacts.taskcluster.net/YSjFxagrQ3OzywXTkL71yQ/0/public/logs/live_backing.log
[task 2017-09-18T18:33:50.138Z] 18:33:50 INFO - Running post-action listener: _package_coverage_data
[task 2017-09-18T18:33:50.138Z] 18:33:50 FATAL - Could not find relative topsrcdir in code coverage data!
[task 2017-09-18T18:33:50.139Z] 18:33:50 FATAL - Running post_fatal callback...
[task 2017-09-18T18:33:50.139Z] 18:33:50 FATAL - Exiting -1
![]() |
Assignee | |
Comment 1•8 years ago
|
||
![]() |
Assignee | |
Comment 2•8 years ago
|
||
:gmierz - I'm not very familiar with ccov and it looks like you might be the expert? Would appreciate your thoughts on how best to handle this situation.
My test-verify (TV) test task is designed to sometimes run no tests. It checks, in the mozharness script, to see if the changeset has modified any test files that it cares about; if the changeset has modified such files, tests are run, but if not, then the script just exits without running any tests. When no tests are run on ccov, I run into this error. I suppose ccov expects some tests to be run / the browser to have been started at least once / something like that?
If nothing else, I can exclude test-verify from running on ccov, but I wonder if there is a better solution.
Flags: needinfo?(gmierz2)
Comment 3•8 years ago
|
||
Hi :gbrown, I'm happy to help! This error comes from here: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/testing/codecoverage.py?q=Could+not+find+relative+topsrcdir&redirect_type=single#111
The code here (in the for loop above) is trying to find the top level source directory from a given temporary directory that stores the coverage '.gcda' files. However, because no gcda files were created, there was no top-level source directory created also.
My first idea to get around this problem would be to add a command at [1] and use that on line 111 to check if it's OK that there is no dir and then exit early. If the TV test task is defined in something like [2], then you could add the command to the mozharness extra-options, but only use it on linux64-ccov. Or, you can add it through the transform given here at [3]. What do you think?
So far, this is the only way that I can think of to get around this problem. We could just skip that part and exit early if there is no directory regardless of the flag, but this error has come in handy at least twice (from what I recall) so I'm a little hesitant on removing it.
[1]: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/testing/codecoverage.py?q=Could+not+find+relative+topsrcdir&redirect_type=single#16
[2]: https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/test/tests.yml
[3]: https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/tests.py#624
Flags: needinfo?(gmierz2)
![]() |
Assignee | |
Comment 4•8 years ago
|
||
Thanks for your advice. I certainly could add a new option as you suggest, but I wonder if this would be reasonable instead: use the existing --disable-ccov-upload.
test-verify runs only when tests are modified, and then only runs the modified tests -- typically just one, or a few individual tests -- and it runs those modified tests over an over. It's so different from our regular testing, I wonder if we really want to collect the data from test-verify?
Either way is fine with me: Let me know what you think is best.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=00eb2bafb7e6d08645cca4522e3773d5c94b5f62&filter-tier=1&filter-tier=2&filter-tier=3
Attachment #8909992 -
Flags: review?(gmierz2)
Comment 5•8 years ago
|
||
Comment on attachment 8909992 [details] [diff] [review]
use --disable-ccov-upload for test-verify
Review of attachment 8909992 [details] [diff] [review]:
-----------------------------------------------------------------
This will be fine because it sounds lile you're running tests that are in other test suites that are being run with code coverage any way.
I'm just going to ask for a second opinion from jmaher in case we'd like coverage from this for some reason that I'm missing.
:jmaher, I think it's perfectly fine if we disable code coverage on this test suite, do you have any thoughts?
Attachment #8909992 -
Flags: review?(gmierz2) → review+
Comment 7•8 years ago
|
||
:gbrown, if you want to, you could even disable this test suite from running on ccov completely through the transform I gave in comment 3 link [3] (or another way if you know of one) which I think may be the best route with the advantage of conserving some resources.
![]() |
Assignee | |
Comment 9•8 years ago
|
||
(In reply to Greg Mierzwinski [:gmierz] from comment #7)
> :gbrown, if you want to, you could even disable this test suite from running
> on ccov completely through the transform I gave in comment 3 link [3] (or
> another way if you know of one) which I think may be the best route with the
> advantage of conserving some resources.
I think you are right: If we are not going to collect the data, why run test-verify on ccov at all?
I think this is the best way to do that -- similar to not running awsy on devedition.
Attachment #8909992 -
Attachment is obsolete: true
Attachment #8910299 -
Flags: review?(gmierz2)
Comment 10•8 years ago
|
||
Comment on attachment 8910299 [details] [diff] [review]
do not run test-verify on ccov
Review of attachment 8910299 [details] [diff] [review]:
-----------------------------------------------------------------
This is much nicer and simpler than what I was thinking. Thanks!
Attachment #8910299 -
Flags: review?(gmierz2) → review+
Comment 11•8 years ago
|
||
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/454abbc380ed
Do not run test-verify on linux64-ccov; r=gmierz
Comment 12•8 years ago
|
||
:gbrown, sorry for the last minute feedback, but I just realized this will most likely break on the mozilla-central branch because we mass-set the run on projects flag in the transform. Do you want to wait and see if this does indeed happen?
![]() |
Assignee | |
Comment 13•8 years ago
|
||
Sigh.
I don't think it will break...it will just be ineffective, because the transform will over-write the test's 'run-on-projects'...right?
Comment 14•8 years ago
|
||
Yes, that's what I expect to happen also, I'm very sorry I didn't catch this before. :(
IMO, the best thing to do here would be to disable the test suite through the transform or some other way that I can't think of.
![]() |
Assignee | |
Comment 15•8 years ago
|
||
I think this should work.
Let's leave the original patch, let it land since it does no harm, and add this on top.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=299bf0eefaf3dc997470a3fe7a0d5041fcd310ed&filter-tier=1&filter-tier=2&filter-tier=3
Attachment #8910501 -
Flags: review?(gmierz2)
![]() |
Assignee | |
Updated•8 years ago
|
Keywords: leave-open
Comment 16•8 years ago
|
||
Comment on attachment 8910501 [details] [diff] [review]
do not run test-verify on ccov, really
Review of attachment 8910501 [details] [diff] [review]:
-----------------------------------------------------------------
This is perfect now, thank you!
Attachment #8910501 -
Flags: review?(gmierz2) → review+
Comment 17•8 years ago
|
||
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e024769dac36
Follow-up: Skip test-verify on linux64-ccov; r=gmierz
Comment 19•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 20•8 years ago
|
||
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/f8dd3f21e434
Follow-up: Skip test-verify on linux64-ccov; r=gmierz a=merge
![]() |
||
Comment 21•8 years ago
|
||
bugherder |
You need to log in
before you can comment on or make changes to this bug.
Description
•